Supporting "where" for unary operations #1061

ipdemes · 2023-10-18T17:10:52Z

No description provided.

…python side

…C++ side

…mean

ipdemes · 2023-10-18T17:12:06Z

This PR also implements nanmean

src/cunumeric/unary/scalar_unary_red_template.inl

src/cunumeric/unary/scalar_unary_red.h

src/cunumeric/unary/scalar_unary_red_template.inl

cunumeric/array.py

Jacobfaib · 2023-11-09T13:36:13Z

src/cunumeric/unary/scalar_unary_red_template.inl

 struct ScalarUnaryRed {
+  ScalarUnaryRed(ScalarUnaryRedArgs& args) { assert(false); }


What is this trying to do? Bar the use of the general template? Or bar a non-const copy? If the point is to deny the use of the general template, prefer doing something like

template <typename T> struct Foo { // Note tautological expression still dependent on template parameter static_assert(!std::is_same_v<T, T>); };

If you'd like to instead bar the use of copy ctor, prefer the usual = delete:

template <typename T> struct Foo { Foo(const Foo&) = delete; };

Thank you, @Jacobfaib . This is not necessary anymore. the purpose for this was to "prevent the use of general template"

src/cunumeric/unary/scalar_unary_red_template.inl

Jacobfaib · 2023-11-09T13:54:25Z

src/cunumeric/unary/unary_red_omp.cc

+    for (size_t o_idx = 0; o_idx < split.outer; ++o_idx)
+      for (size_t i_idx = 0; i_idx < split.inner; ++i_idx) {
+        auto point    = splitter.combine(o_idx, i_idx, rect.lo);
+        auto identity = LG_OP::identity;


You can put this in the if

Jacobfaib · 2023-11-09T13:55:22Z

src/cunumeric/unary/unary_red_omp.cc

+    auto split = splitter.split(rect, collapsed_dim);
+
+#pragma omp parallel for schedule(static)
+    for (size_t o_idx = 0; o_idx < split.outer; ++o_idx)


Minor nit, but consider adding the {} around the for's body. The code is correct as written, just makes it slightly easier to read.

cunumeric/module.py

Co-authored-by: Jacob Faibussowitsch <[email protected]>

Co-authored-by: Manolis Papadakis <[email protected]>

copy-pr-bot · 2023-11-17T21:23:05Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

ipdemes · 2023-11-17T21:51:28Z

But by doing it this way, the summation for np.int64 arrays happens using an intermediate of type np.int64, whereas the NumPy docs specifically note that "float64 intermediate and return values are used for integer inputs".

If this is important for the SLAC usecase, then I think the better solution would be to extend the template of UnaryRedImplBody with two "CODE" template parameters, one for the RHS (input array) type and one for the LHS (accumulator array) type. Then we can compute the sum of an np.int64 array directly into an array of np.float64 type, without converting the input to np.float64.

Makes sense, I will add back dtype to sum and create an issue to support 2 different types for RHS and LHS in UnaryImplBody

manopapad · 2023-11-18T01:06:23Z

/ok to test

for more information, see https://pre-commit.ci

manopapad · 2023-11-30T16:03:09Z

/ok to test

manopapad

One minor comment, otherwise looks good

manopapad · 2023-12-02T02:37:41Z

cunumeric/array.py

            divisor = np.array(divisor, dtype=sum_array.dtype)  # type: ignore [assignment] # noqa
+
        if dtype.kind == "f" or dtype.kind == "c":


I think you can just get rid of the dtype parameter at this point, and just use sum_array.dtype everywhere in this function.

ipdemes · 2023-12-02T04:09:15Z

/ok to test

ipdemes added 17 commits October 17, 2023 09:57

adding support for adding where to unary reduction operations on the …

f0eef07

…python side

adding support for adding where to unary reduction operations on the …

a93748d

…C++ side

formatting

79c7aca

fixing error when calculation divisor

a64e829

fixing error with mean(where)

6f7b059

updating test

d12fe55

adding logic for where in unary_red on C++

ec41b52

towards improving data usage by mean

9128530

debugging

76dc871

removing debug output

cbe93d9

adding support for broadcasting where + initial implementation of nan…

419a5af

…mean

adding test for nanmean

0d7d66c

some clean-up + formatting

baf61ed

fixing issues after rebase

30b1e18

code clean-up

2277234

code clean-up

73e0609

adding test_where for unary operations

dfafdcc

ipdemes requested a review from manopapad October 18, 2023 17:10

ipdemes self-assigned this Oct 18, 2023

ipdemes added the category:new-feature PR introduces a new feature and will be classified as such in release notes label Oct 18, 2023

ipdemes added 3 commits October 18, 2023 10:51

adding nanmean to docs

3071edd

nanmean is not oficiall method of ndarray

67aab76

fixing test

09a4477

manopapad requested changes Nov 9, 2023

View reviewed changes

Jacobfaib reviewed Nov 9, 2023

View reviewed changes

marcinz changed the base branch from branch-23.11 to branch-24.01 November 9, 2023 17:11

ipdemes and others added 3 commits November 9, 2023 10:57

Update src/cunumeric/unary/scalar_unary_red_template.inl

851dd42

Co-authored-by: Jacob Faibussowitsch <[email protected]>

Update src/cunumeric/unary/scalar_unary_red_template.inl

83308a6

Co-authored-by: Jacob Faibussowitsch <[email protected]>

Update src/cunumeric/unary/scalar_unary_red_template.inl

df70b28

Co-authored-by: Jacob Faibussowitsch <[email protected]>

ipdemes and others added 12 commits November 17, 2023 10:35

Update src/cunumeric/unary/unary_red.cu

a51135d

Co-authored-by: Manolis Papadakis <[email protected]>

Update src/cunumeric/unary/unary_red.cc

e703cc9

Co-authored-by: Manolis Papadakis <[email protected]>

Update src/cunumeric/unary/unary_red_omp.cc

7ea110f

Co-authored-by: Manolis Papadakis <[email protected]>

Update src/cunumeric/unary/scalar_unary_red_template.inl

f080a95

Co-authored-by: Manolis Papadakis <[email protected]>

Update src/cunumeric/unary/scalar_unary_red_template.inl

c039cb2

Co-authored-by: Manolis Papadakis <[email protected]>

Update src/cunumeric/unary/unary_red_template.inl

6896b4e

Co-authored-by: Manolis Papadakis <[email protected]>

Update src/cunumeric/unary/scalar_unary_red_template.inl

4014239

Co-authored-by: Manolis Papadakis <[email protected]>

Update src/cunumeric/unary/unary_red_template.inl

fe84bb1

Co-authored-by: Manolis Papadakis <[email protected]>

Update src/cunumeric/unary/unary_red.cu

70fa320

Co-authored-by: Manolis Papadakis <[email protected]>

Update cunumeric/array.py

9d5b364

Co-authored-by: Manolis Papadakis <[email protected]>

Update cunumeric/array.py

2bd62eb

Co-authored-by: Manolis Papadakis <[email protected]>

fixing bug in cuda code

f504f05

ipdemes added 2 commits November 17, 2023 13:56

adding dtype back to sum

b15f656

removing ignore_nan

7a77535

ipdemes requested a review from manopapad November 17, 2023 22:26

manopapad approved these changes Nov 21, 2023

View reviewed changes

ipdemes added 2 commits November 21, 2023 12:37

Merge remote-tracking branch 'origin/branch-24.01' into unary_red_where

b39364b

fixing var for the case when where is not None

bdb49e9

ipdemes force-pushed the unary_red_where branch from 716e0d6 to bdb49e9 Compare November 30, 2023 05:14

[pre-commit.ci] auto fixes from pre-commit.com hooks

4ef0749

for more information, see https://pre-commit.ci

ipdemes added 2 commits November 30, 2023 09:38

fixing eager logic for var

18a6fe8

Merge remote-tracking branch 'origin/branch-24.01' into unary_red_where

7c1fdaa

manopapad approved these changes Dec 2, 2023

View reviewed changes

removing dtype from _normalize_summation

869b176

ipdemes merged commit 672e8c6 into nv-legate:branch-24.01 Dec 3, 2023
22 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Supporting "where" for unary operations #1061

Supporting "where" for unary operations #1061

ipdemes commented Oct 18, 2023

ipdemes commented Oct 18, 2023

Jacobfaib Nov 9, 2023

ipdemes Nov 14, 2023

Jacobfaib Nov 9, 2023

Jacobfaib Nov 9, 2023

copy-pr-bot bot commented Nov 17, 2023

ipdemes commented Nov 17, 2023

manopapad commented Nov 18, 2023

manopapad commented Nov 30, 2023

manopapad left a comment

manopapad Dec 2, 2023

ipdemes commented Dec 2, 2023

		struct ScalarUnaryRed {
		ScalarUnaryRed(ScalarUnaryRedArgs& args) { assert(false); }

		divisor = np.array(divisor, dtype=sum_array.dtype) # type: ignore [assignment] # noqa

		if dtype.kind == "f" or dtype.kind == "c":

Supporting "where" for unary operations #1061

Supporting "where" for unary operations #1061

Conversation

ipdemes commented Oct 18, 2023

ipdemes commented Oct 18, 2023

Jacobfaib Nov 9, 2023

Choose a reason for hiding this comment

ipdemes Nov 14, 2023

Choose a reason for hiding this comment

Jacobfaib Nov 9, 2023

Choose a reason for hiding this comment

Jacobfaib Nov 9, 2023

Choose a reason for hiding this comment

copy-pr-bot bot commented Nov 17, 2023

ipdemes commented Nov 17, 2023

manopapad commented Nov 18, 2023

manopapad commented Nov 30, 2023

manopapad left a comment

Choose a reason for hiding this comment

manopapad Dec 2, 2023

Choose a reason for hiding this comment

ipdemes commented Dec 2, 2023